fix(shell-hook): honor explicit manifest#4536
Merged
tdejager merged 2 commits intoprefix-dev:mainfrom Sep 10, 2025
Merged
Conversation
lucascolley
added
bug
tdejager
reviewed
Sep 8, 2025
remimimimimi
force-pushed
the
fix/shell-hook-manifest-path
branch
from
54ea693 to
afa966b
Compare
lucascolley
approved these changes
Sep 8, 2025
Collaborator
lucascolley
left a comment
lucascolley
left a comment
There was a problem hiding this comment.
fix confirmed, nice work @remimimimimi ! What a small diff :)
pixi on 🎋 fix/shell-hook-manifest-path via 🐍 via 🦀 v1.86.0 via 🧚 v0.54.1
❯ pixi init --no-progress "/Users/lucascolley/sandbox/pixi4149"
✔ Created /Users/lucascolley/sandbox/pixi4149/pixi.toml
pixi on 🎋 fix/shell-hook-manifest-path via 🐍 via 🦀 v1.86.0 via 🧚 v0.54.1
❯ eval "$(pixid shell-hook --manifest-path "/Users/lucascolley/sandbox/pixi4149/pixi.toml")"
pixi on 🎋 fix/shell-hook-manifest-path via 🐍 via 🦀 v1.86.0 via 🧚 v0.54.1 (default)
❯ pixid project channel add --manifest-path "/Users/lucascolley/sandbox/pixi4149/pixi.toml" --prepend bioimageit
✔ Added bioimageit (https://conda.anaconda.org/bioimageit/)
pixi on 🎋 fix/shell-hook-manifest-path via 🐍 via 🦀 v1.86.0 via 🧚 v0.54.1 (default)
❯ pixid add --manifest-path "/Users/lucascolley/sandbox/pixi4149/pixi.toml" atlas
✔ Added atlas >=1.0.3,<2
pixi on 🎋 fix/shell-hook-manifest-path via 🐍 via 🦀 v1.86.0 via 🧚 v0.54.1 (default)
❯ which atlas
/Users/lucascolley/sandbox/pixi4149/.pixi/envs/default/bin/atlas
Collaborator
|
Maybe we should add that test case to the test suite before merging? |
Contributor
|
That's always prudent indeed 🙂 |
remimimimimi
force-pushed
the
fix/shell-hook-manifest-path
branch
from
a6059ad to
70371db
Compare
tdejager
reviewed
Sep 9, 2025
Comment on lines
+7
to
+20
| struct CwdGuard(std::path::PathBuf); | ||
| impl CwdGuard { | ||
| fn new(new_dir: std::path::PathBuf) -> CwdGuard { | ||
| let orig_dir = std::env::current_dir().unwrap(); | ||
| let guard = CwdGuard(orig_dir); | ||
| std::env::set_current_dir(new_dir).unwrap(); | ||
| guard | ||
| } | ||
| } | ||
| impl Drop for CwdGuard { | ||
| fn drop(&mut self) { | ||
| let _ = std::env::set_current_dir(&self.0); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Just inline this into the code. Also add a comment that tests should not be added to this file to interefere with the cwd setting :)
Contributor
There was a problem hiding this comment.
We just discussed, so I suppose its either a seperate binary. Using subprocess in the rust test (ugly :P). Or a pytest.
remimimimimi
force-pushed
the
fix/shell-hook-manifest-path
branch
from
70371db to
705264c
Compare
remimimimimi
force-pushed
the
fix/shell-hook-manifest-path
branch
from
705264c to
2b7494c
Compare
tdejager
approved these changes
Sep 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Should resolve #4149.
Test: